Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix usage vcpkg for windows and add cmake preset for vscode #414

Draft
wants to merge 32 commits into
base: headless-mode
Choose a base branch
from

Conversation

Ygrik2003
Copy link

@Ygrik2003 Ygrik2003 commented Dec 14, 2024

Добавил пресет для интеграции с VSCode и подправил cmake для правильной работы vcpkg

Теперь не нужно 2 раза вызывать конфигурацию. Избавился от параметра VOXELENGINE_BUILD_WINDOWS_VCPKG в CMake. Теперь по дефолту windows билд собирается с vcpkg

Избавился от vcpkg загрузки из CMake, теперь есть ограничения:

  1. Должна быть выставлена переменная среды VCPKG_ROOT, указывающая на корень установки vcpkg
  2. Собственно сам vcpkg нужно ставить самому как в доке

За одно подправил деплой ресов, теперь он всегда будет работать под все платформы без проблем и костылей

Единственное, не уверен, что не сломал сборку windows-clang.yml, нужно перепроверить, возможно нужно делать экспорт полного пути к vcpkg

.github\workflows\windows-clang.yml:46

export VCPKG_ROOT=./vcpkg
export VCPKG_DEFAULT_TRIPLET=x64-mingw-static
export VCPKG_DEFAULT_HOST_TRIPLET=x64-mingw-static
mkdir build
cd build
cmake --preset default-windows -G "MinGW Makefiles" -DVCPKG_TARGET_TRIPLET=x64-mingw-static -DCMAKE_BUILD_TYPE=Release ..
cmake --build . --config Release

В любом случае нужно перепроверить этот билд

@Ygrik2003
Copy link
Author

К слову, потыкался чутка с vcpkg, оказывается, если запускаться в дев терминале вижлы, то там прокидывается переменная VCPKG_ROOT автоматически, так что можно еще будет дописать в readme, вечером закомитаю

@MihailRis
Copy link
Owner

Обновил настройки workflow в ветке, чтобы запускались проверки в PR. Нужно вытянуть изменения из headless-mode.

Ygrik2003 and others added 3 commits December 20, 2024 15:39
…code' of github.com:Ygrik2003/VoxelEngine-Cpp into Fix-usage-vcpkg-for-windows-and-add-cmake-preset-for-vscode
@Ygrik2003 Ygrik2003 marked this pull request as draft December 20, 2024 14:11
@Ygrik2003
Copy link
Author

Еще немного потрогал Github Actions, теперь билды будут работать прямо из пресетов, что должно чутка облегчить работу с workflows при работе с флагами
Сделал clang сборку через генератор Ninja Multi-Config
Переделал подключение vcpkg в github actions в соответствии с докой
в общем нужно перепроверить билды

@Ygrik2003 Ygrik2003 marked this pull request as ready for review December 20, 2024 14:23
@Ygrik2003
Copy link
Author

image

@Ygrik2003
Copy link
Author

Ygrik2003 commented Dec 20, 2024

Отвалились ctest и неправильные пути для msys2 под clang, завтра-послезавтра гляну

@Ygrik2003 Ygrik2003 marked this pull request as draft December 20, 2024 22:11
@Ygrik2003
Copy link
Author

Короче можно попробовать первоначальный фикс с VCPKG_ROOT: ./vcpkg
если не сработает, то нужно будет думать

@Ygrik2003 Ygrik2003 marked this pull request as ready for review December 20, 2024 22:35
@Ygrik2003 Ygrik2003 marked this pull request as draft December 21, 2024 07:22
@Ygrik2003 Ygrik2003 marked this pull request as ready for review December 21, 2024 22:00
@Ygrik2003 Ygrik2003 marked this pull request as draft December 21, 2024 23:08
@Ygrik2003 Ygrik2003 marked this pull request as ready for review December 22, 2024 01:02
@Ygrik2003
Copy link
Author

Короче намучался с MSYS2, судя по всему, он нужен был для того, что бы cmake по умолчанию находил clang. Так как я дописал cmake и теперь компилятор выбирается пресетом, я выкосил msys2 из windows-clang и теперь как и обычный windows конфиг, собирается через pwsh
Если в целом возражений нет, то ПР готов

run: |
cp $env:MSYS2_LOCATION/clang64/bin/lua51.dll ${{ github.workspace }}/packaged/
cp -r build/* packaged/
cp C:/Windows/System32/msvcp140.dll packaged/Release/msvcp140.dll
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

К чему в Clang сборке эта строка? (50)
Кроме того, линковка теперь динамическая, при том, что должна быть статическая.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

В релизе отсутствует vctest.exe

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Внутри, вместо VoxelCore.exe лежит VoxelEngine.exe, несмотря на строку 51.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Кроме того, exe имеет динамическую зависимость в msvcp140.dll, которой в Clang-сборке быть не должно

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

По поводу msvcp140, это как раз таки одна из особенностей отказа от MSYS2
Дело в том, что msys2 и Cygwin поставляют свой Clang, который зависит от своих либ
Тот Clang, который я заюзал (LLVM), он юзает такой же рантайм, как и msvc. Что касательно линковки, то необходимый набор dll'ок слегка отличается, но msvcp140.dll все же присутствует
Нашел такую статью, которая частично объясняет происходящее.

А вообще, если у человека не установлен microsoft visual c++ redistributable (обычно устанавливается автоматически при скачивании некоторых игр в стиме), то игра вообще не запустится

Copy link
Author

@Ygrik2003 Ygrik2003 Dec 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Поправил остальное, деплоилась не та папка

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Основной вопрос в статической линковке, что была реализована в workflow ранее, где исключением был только lua51.dll.

@Ygrik2003 Ygrik2003 marked this pull request as draft December 22, 2024 11:03
@Ygrik2003 Ygrik2003 marked this pull request as ready for review December 22, 2024 13:28
@Ygrik2003
Copy link
Author

@MihailRis А зачем вообще нужно переименование VoxelEngine в VoxelCore?

@Ygrik2003 Ygrik2003 requested a review from MihailRis December 22, 2024 13:32
@Ygrik2003 Ygrik2003 marked this pull request as draft December 24, 2024 10:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants